Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1419, Resolve sequence count auto-increment rollover bug #1482

Merged

Conversation

skliper
Copy link
Contributor

@skliper skliper commented May 4, 2021

Describe the contribution
Fix #1419 - Adds CFE_MSG_GetNextSequenceCount so the auto-increment of the local sequence counter works when sending tlm (and increment is enabled). Updates unit tests and adds the old style stub. The unit tests check for the correct rollover behavior.

Note - Will need to rebase when autogenerated stubs are merged, will update at that point.

Testing performed
Build and execute unit tests, passed

Expected behavior changes
Sequence count will roll over based on the mask. Before the fix the sequence counter would "stick" in telemetry until the passed in value rolled over.

System(s) tested on

  • Hardware: Local docker
  • OS: Ubuntu 18.04
  • Versions: cFS Bundle main + this commit

Additional context
Will need to deconflict once autogenerated stubs are merged, #1463. Note this just fixes the reported bug, and doesn't add any of the other extra functionality discussed as part of #1419.

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

@skliper skliper added this to the 7.0.0 milestone May 4, 2021
@skliper skliper added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 4, 2021
@astrogeco
Copy link
Contributor

CCB:2021-05-05 APPROVED

  • Open new issue to update stub style

@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB conflicts and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels May 12, 2021
@astrogeco
Copy link
Contributor

CCB:2021-05-12 APPROVED

@astrogeco
Copy link
Contributor

@skliper please fix conflicts when you can

@skliper skliper force-pushed the fix1419-get_next_seqcnt branch 2 times, most recently from 7d65dca to fb344b7 Compare May 12, 2021 21:21
@skliper
Copy link
Contributor Author

skliper commented May 12, 2021

@astrogeco - fixed, unit tests built and passed, ready to merge! (priority for stakeholder)

@astrogeco astrogeco changed the base branch from main to integration-candidate May 12, 2021 21:57
@astrogeco astrogeco merged commit 56ca8b3 into nasa:integration-candidate May 12, 2021
@astrogeco
Copy link
Contributor

astrogeco commented May 19, 2021

Regarding the note in the context section, are any of the remaining features discussed in #1419 being encapsulated into separate issues?

astrogeco added a commit to nasa/cFS that referenced this pull request May 19, 2021
nasa/cFE#1482, Resolve sequence count auto-increment rollover bug

nasa/osal#985, rename hooks to handlers
nasa/osal#1000, propagate status code in OS_rmdir
nasa/osal#1001, rework "unit-tests" to use macros
nasa/osal#1003, remove extra newlines in utassert logs
nasa/osal#990, Add test for object id inline functions
nasa/osal#998, fixed invalid inputs for OS_mkdir
nasa/osal#812, Improves config guide documentation
@skliper
Copy link
Contributor Author

skliper commented May 19, 2021

Regarding the note in the context section, are any of the remaining features discussed in #1419 being encapsulated into separate issues?

No. There aren't any onboard sequence count verification requirements (or related design/implementation) at this point. We can certainly consider it if there is a stakeholder that wants to propose new requirements that aren't satisfied using the current APIs, but at this point it's probably premature.

astrogeco added a commit to nasa/cFS that referenced this pull request May 19, 2021
Combines:

nasa/cFE#1508, cFE v6.8.0-rc1+dev580
nasa/osal#1006, osal v5.1.0-rc1+dev452

Includes:

nasa/cFE#1482, Resolve sequence count auto-increment rollover bug
nasa/cFE#1491, Correctly format code block section terminator
nasa/cFE#1530, Fix typos in developer guide

nasa/osal#985, rename hooks to handlers
nasa/osal#1000, propagate status code in OS_rmdir
nasa/osal#1001, rework "unit-tests" to use macros
nasa/osal#1003, remove extra newlines in utassert logs
nasa/osal#990, Add test for object id inline functions
nasa/osal#998, fixed invalid inputs for OS_mkdir
nasa/osal#812, Improves config guide documentation
nasa/osal#987, Show CodeQL Preview
astrogeco added a commit to nasa/cFS that referenced this pull request May 19, 2021
Combines:

nasa/cFE#1508, cFE v6.8.0-rc1+dev580
nasa/osal#1006, osal v5.1.0-rc1+dev452

Includes:

nasa/cFE#1482, Resolve sequence count auto-increment rollover bug
nasa/cFE#1491, Correctly format code block section terminator
nasa/cFE#1530, Fix typos in developer guide

nasa/osal#985, rename hooks to handlers
nasa/osal#1000, propagate status code in OS_rmdir
nasa/osal#1001, rework "unit-tests" to use macros
nasa/osal#1003, remove extra newlines in utassert logs
nasa/osal#990, Add test for object id inline functions
nasa/osal#998, fixed invalid inputs for OS_mkdir
nasa/osal#812, Improves config guide documentation
nasa/osal#987, Show CodeQL Preview

Co-Authored-By: Jake Hageman <skliper@users.noreply.github.com>
Co-Authored-By: Joseph Hickey <joseph.p.hickey@nasa.gov>
Co-Authored-By: Ariel Adams <ArielSAdamsNASA@users.noreply.github.com>
Co-Authored-By: Alex Campbell <zanzaben@users.noreply.github.com>
Co-Authored-By: Tobias Nießen <tniessen@users.noreply.github.com>
Co-Authored-By: Jonathan Bohren <jbohren-hbr@users.noreply.github.com>
Co-Authored-By: Andrei Tumbar <Kronos3@users.noreply.github.com>
@skliper skliper deleted the fix1419-get_next_seqcnt branch May 25, 2021 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto Increment Telemetry Sequence Count Overflow
2 participants